fix: prevent duplicate LM Studio models in provider configuration (#6954) #6956
Conversation
- Use consistent model.path as key for both downloaded and loaded models - Fixes issue where models appeared twice with different identifiers - Update tests to reflect the new consistent key usage Fixes #6954
dosubot
bot
added
size:XS
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| for (const lmstudioModel of loadedModels) { | ||
| models[lmstudioModel.modelKey] = parseLMStudioModel(lmstudioModel) | ||
| modelsWithLoadedDetails.add(lmstudioModel.modelKey) | ||
| // Use path as the key to avoid duplicates when a model is both downloaded and loaded |
There was a problem hiding this comment.
Good fix! The comment clearly explains why we're using path as the key. This should resolve the duplicate model issue reported in #6954.
| modelsWithLoadedDetails.add(lmstudioModel.modelKey) | ||
| // Use path as the key to avoid duplicates when a model is both downloaded and loaded | ||
| models[lmstudioModel.path] = parseLMStudioModel(lmstudioModel) | ||
| modelsWithLoadedDetails.add(lmstudioModel.path) |
There was a problem hiding this comment.
Nice consistency - updating the Set to track paths instead of modelKeys aligns with the new keying strategy.
|
|
||
| const expectedParsedModel = parseLMStudioModel(mockRawModel) | ||
| expect(result).toEqual({ [mockRawModel.modelKey]: expectedParsedModel }) | ||
| expect(result).toEqual({ [mockRawModel.path]: expectedParsedModel }) |
There was a problem hiding this comment.
Test correctly updated to expect the path as the key instead of modelKey. Good to see the test coverage maintained.
hannesrudolph
added
the
Issue/PR - Triage
|
LGTM. |
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
|
Closing this PR to re-scope the solution. The better approach is to only show loaded models from LM Studio, which completely eliminates duplicates and ensures only usable models are displayed. |
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
|
@daniel-lxs Just to chime in: only showing loaded models would IMO be a really bad idea. Since LMStudio has JIT-model-loading, in many cases "showing only loaded models" would amount to showing no models at all. The way most people use LMStudio (me included) is they keep a repository of available models and load them on demand according to the needs. I think the fix is actually quite decent, since the problem was showing the loaded and available models using a different key. |
Summary
This PR fixes an issue where LM Studio models appeared twice in the Provider Configuration Profile - once with the model path and once with the model id.
Problem
When a model is both downloaded and loaded in LM Studio, it was being added to the models list twice with different keys:
model.pathas the keymodel.modelKeyas the keyThis caused the same model to appear as duplicate entries in the UI.
Solution
Use
model.pathconsistently as the key for both downloaded and loaded models. This ensures each model appears only once in the configuration profile, regardless of whether it's just downloaded or also loaded.Changes
getLMStudioModels()insrc/api/providers/fetchers/lmstudio.tsto uselmstudioModel.pathinstead oflmstudioModel.modelKeyfor loaded modelssrc/api/providers/fetchers/__tests__/lmstudio.test.tsto expect the path as the keyTesting
Fixes #6954
Important
Fixes duplicate LM Studio models in Provider Configuration by using
model.pathas the key for both downloaded and loaded models.model.pathas the key for both downloaded and loaded models ingetLMStudioModels()inlmstudio.ts.lmstudio.test.tsto expectmodel.pathas the key.This description was created by
for 21c34f1. You can customize this summary. It will automatically update as commits are pushed.